Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweaks, a convenience method, and speculative refactor. #1

Merged
merged 5 commits into from May 23, 2012

Conversation

hypomodern
Copy link
Contributor

Just taking a look at what is going on here.

I have questions. It seems like dick-all is done with the nodes specified in the template. They look like they set up a state machine governing how winners move around but this data this thrown away by the Template and Node doesn't implement any state logic. Is this to-do, or does starleagues have it and you haven't been able to extract it yet?

Do you want or need help with implementing that, is my real question, I guess.

@@ -77,10 +77,14 @@ def seed players
end

# This needs refactoring. Inconsistent interface
# Not sure if this is what you want here or not, but this now accepts hashlike input
# I'm also unsure about a good default position. I assume 1? - MHW
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 will never be the correct default position because the winner is the root node, which must be n+1 where n is the highest position value on the left 'half' of the tree.

@root.payload = winner is probably sufficient for now, because I still have some math to do before I can implement automatic recalculation of node positions to map to a given binary tree representation, and I think that's a bit overkill for initial version.

@cadwallion
Copy link
Contributor

I would rather not assume a Hash as the node positions. I'm putting in empty hashes for the placeholders mainly because it's 1) easily serializable 2) less intense than some arbitrary object. However, I don't want to pigeon-hole the content into just a hash. SC2Battles, for example, uses a Seat object in the positions to carry extra info and to add AM::Serializer support.

Everything else looks good.

@hypomodern
Copy link
Contributor Author

There ya go.

@cadwallion
Copy link
Contributor

👍 🎉

cadwallion added a commit that referenced this pull request May 23, 2012
Tweaks, a convenience method, and speculative refactor.
@cadwallion cadwallion merged commit 68be8fc into master May 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants